Skip to content

Keep ReconnectApp incremental for signed-in users so other devices don't lose chat messages - #102247

Merged
Julesssss merged 9 commits into
mainfrom
claude-reconnectAppIncrementalForSignedInUsers
Oct 14, 2026
Merged

Julesssss merged 9 commits into
mainfrom
claude-reconnectAppIncrementalForSignedInUsers

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

The server sends the reconnectApp Pusher event on the account-wide channel, so every signed-in device of the account receives it, not only the device whose updateIDFrom is missing. For signed-in users, the handler always called reconnectApp() with no argument. That triggers a full ReconnectApp, which skips the "older than our state" guard and replaces the report-action collections. As a result, when a third device signed in, the two devices that were already chatting lost messages they held.

This PR makes the handler pass lastUpdateIDAppliedToClient for all users, as it already did for anonymous users (#97847). Recovery still works:

  • If the server can't serve the requested range, it still returns full data.
  • If the client has no lastUpdateIDAppliedToClient yet, reconnectApp receives undefined and still does a full reconnect.

Unit tests in tests/actions/UserTest.ts now cover the signed-in incremental case and the no-watermark fallback.

Open backend questions from the proposal (not addressed here, need an internal engineer): whether reconnectApp should be scoped to the affected device instead of broadcast account-wide, and confirmation that a full ReconnectApp uses setcollection for reportActions_.

AI tests run locally: npm run lint-changed ✅, npm run typecheck ✅, npm run spell-changed ✅, npm test -- tests/actions/UserTest.ts tests/actions/AppTest.ts ✅ (57 passed). The full Jest suite, full npm run lint, formatter check, and Storybook smoke test were not run locally; CI covers them.

Fixed Issues

$ #100791
PROPOSAL: #100791 (comment)

Tests

  1. Sign in to the same test account in two sessions. Open the same chat, send messages from both sessions, and verify they appear.
  2. Sign in to that account in a third session. Verify the earlier messages remain in the first two sessions after 30 seconds.
  3. Send another message, then reload the chat. Verify the earlier and new messages remain.
  4. Repeat with Android native, Android mWeb Chrome, iOS native, iOS mWeb Safari, and macOS Chrome as the existing session.
  5. Verify a fresh session still loads its chat.
  6. Send a message containing a URL, inline code, a code block, a quote, a heading, bold, strikethrough, and italic text. Reopen the chat and verify its history and formatting remain.
  • Verify that no errors appear in the JS console.

Offline tests

  1. Open the chat online, then disconnect the network or enable Account → Troubleshoot → Force offline.
  2. Verify existing messages remain visible and a new message queues locally.
  3. Reconnect, reload the chat, and verify the earlier and queued messages remain.

QA Steps

Follow the Tests and Offline tests above.

  • Verify that no errors appear in the JS console.

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
PR-102247-android-native-online-redacted.mp4
Android: mWeb Chrome
PR-102247-android-mweb-chrome-online-redacted.mp4
iOS: Native
PR-102247-ios-native-online-redacted.mp4
iOS: mWeb Safari
PR-102247-ios-mweb-safari-online-redacted.mp4
MacOS: Chrome / Safari
PR-102247-desktop-chrome-three-session-online-redacted.mp4

…P Pusher event

Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team September 25, 2026 04:00
@melvin-bot melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Sep 25, 2026
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I created this PR from an upstream branch since I don't have push access to your fork.

To take ownership of this branch and be able to push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-reconnectAppIncrementalForSignedInUsers
git checkout -b claude-reconnectAppIncrementalForSignedInUsers upstream/claude-reconnectAppIncrementalForSignedInUsers
git push -u origin claude-reconnectAppIncrementalForSignedInUsers

Then you can close this PR and open a new one from your fork.

@MelvinBot

MelvinBot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Android test steps verification results: ⚠️ partial

I signed in and sent 4 messages in the Concierge chat. All 4 were still visible after about 30 seconds. I could not sign in to the same account from a second device, so the scenario this PR fixes was not tested. The PR's Tests section is still a TODO, so I used the steps from the linked issue.
All 4 messages still present after about 30 seconds

Step Status
Sign in to the account on Android and open a chat (Concierge)Signed in, completed Join and onboarding, and opened the Concierge chat.
Concierge chat opened after sign-in
✅
Send several messages in the chat; all appearSent 4 test messages. Each one appeared in the chat right after sending.
Three test messages visible in the Concierge chat
✅
Sign in to the same account from another device while Android stays open on the chatNot performed. This run has only one emulator session, and a second sign-in to the same email needs a magic code that I cannot get.
❌
Messages from step 2 stay visible after the other sign-in, including after ~30sAll 4 messages stayed visible after about 30 seconds on the single device. Without the second sign-in, this only confirms the single-device baseline, not the fix.
All 4 messages still present after about 30 seconds
⚠️
No JS errors appear in the logs during the flowLogcat showed no errors while sending messages or during the 30-second wait. During onboarding, before the test started, Onyx recorded "Unexpected error posting the comment" and "Unexpected error creating this chat" on a different report. No error showed in the UI, and the chat under test was not affected.
⚠️

view run · view recording

@MelvinBot

MelvinBot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Web test steps verification results: ⚠️ partial

I sent 3 messages in the Concierge chat. All 3 were still visible after 30 seconds, with no visible errors. I could not sign in to the same account from a second device, so the scenario this PR fixes was not tested. The PR's Tests section is still a TODO, so I used the steps from the linked issue.
All three messages still visible about 30 seconds later

Step Status
Sign in and open a chat (Concierge)Finished onboarding on the signed-in session. The Concierge chat was open and usable afterward.
Concierge chat opened after onboarding
✅
Send several messages in the chat; all appearSent 3 test messages. Each one appeared in the chat right after sending.
All three test messages visible in the Concierge chat
✅
Sign in to the same account from a second device while the first session stays openNot performed. This run has only one browser session, and it can't open a second signed-in session for the same account.
❌
Messages from step 2 stay visible after the other sign-in, including after ~30sAll 3 messages stayed visible after 30 seconds in the single session. Without the second sign-in, this only confirms the single-session baseline, not the fix.
All three messages still visible about 30 seconds later
⚠️
No JS console errors during the flowConsole logs could not be captured in this web session. No error banners, toasts, or broken UI appeared in any screenshot.
⚠️

view run · view recording

@nyomanjyotisa

Copy link
Copy Markdown
Member

@MelvinBot Please merge the latest main into this PR branch.

…crementalForSignedInUsers

Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I merged the latest main into this branch and pushed it. The merge had no conflicts. It brought in 670 commits and added merge commit 9cd3090.


view run

@nyomanjyotisa

Copy link
Copy Markdown
Member

@MelvinBot Could you remove “(anonymous users)” from this comment? It now applies to signed-in users too.

…ient comment

Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I removed "(anonymous users)" from the comment in src/libs/actions/User.ts:89 and pushed commit 5a2106a. This commit changes only that comment.


view run

@nyomanjyotisa

Copy link
Copy Markdown
Member
Updated testing, checklist, and recordings

Tests

  1. Sign in to the same test account in two sessions. Open the same chat, send messages from both sessions, and verify they appear.
  2. Sign in to that account in a third session. Verify the earlier messages remain in the first two sessions after 30 seconds.
  3. Send another message, then reload the chat. Verify the earlier and new messages remain.
  4. Repeat with Android native, Android mWeb Chrome, iOS native, iOS mWeb Safari, and macOS Chrome as the existing session.
  5. Verify a fresh session still loads its chat.
  6. Send a message containing a URL, inline code, a code block, a quote, a heading, bold, strikethrough, and italic text. Reopen the chat and verify its history and formatting remain.
  • Verify that no errors appear in the JS console.

Offline tests

  1. Open the chat online, then disconnect the network or enable Account → Troubleshoot → Force offline.
  2. Verify existing messages remain visible and a new message queues locally.
  3. Reconnect, reload the chat, and verify the earlier and queued messages remain.

QA Steps

Follow the Tests and Offline tests above.

  • Verify that no errors appear in the JS console.

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
PR-102247-android-native-online-redacted.mp4
Android: mWeb Chrome
PR-102247-android-mweb-chrome-online-redacted.mp4
iOS: Native
PR-102247-ios-native-online-redacted.mp4
iOS: mWeb Safari
PR-102247-ios-mweb-safari-online-redacted.mp4
MacOS: Chrome / Safari
PR-102247-desktop-chrome-three-session-online-redacted.mp4

@MelvinBot please replace the sections from ### Tests through ### Screenshots/Videos with the content above.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I updated the PR description. Everything from ### Tests through ### Screenshots/Videos now matches your comment, including the five recordings. I left out the outer "Updated testing, checklist, and recordings" wrapper so the sections show normally. Everything above ### Tests is unchanged.


view run

@nyomanjyotisa

Copy link
Copy Markdown
Member

@MelvinBot please mark this PR ready for review.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I couldn't mark this PR ready for review. The command that does it isn't permitted in my current environment, so the PR is still a draft.

nyomanjyotisa, you're an assignee, so you can do it yourself. Click Ready for review at the bottom of the PR's Conversation tab.


view run

@nyomanjyotisa

Copy link
Copy Markdown
Member

@MelvinBot I can’t mark this PR ready for review because GitHub denied permission. Could you retry?

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I retried, but I still can't mark this PR ready for review. The action is blocked in my environment, so the PR is still a draft.

I was wrong earlier when I said you could do it as an assignee. GitHub only lets the PR author or someone with write access change draft status. You have read access to this repo, so GitHub denied you.

To unblock this, ask an Expensify employee with write access to click Ready for review, for example the assigned internal engineer.


view run

@nyomanjyotisa

Copy link
Copy Markdown
Member

@Julesssss Could you help mark this PR ready for review? Melvin’s attempt was blocked, and I don’t have permission.

@Julesssss
Julesssss marked this pull request as ready for review September 30, 2026 23:03
@Julesssss
Julesssss requested a review from a team as a code owner September 30, 2026 23:03
@melvin-bot
melvin-bot Bot requested review from nyomanjyotisa and removed request for a team September 30, 2026 23:03
@melvin-bot

melvin-bot Bot commented Sep 30, 2026

Copy link
Copy Markdown

@nyomanjyotisa Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot
melvin-bot Bot requested review from garrettmknight and removed request for a team September 30, 2026 23:03
@nyomanjyotisa

Copy link
Copy Markdown
Member

@MelvinBot please merge the latest main, keep all three API command imports when resolving MiddlewareTest.ts, and format SaveResponseInOnyx.ts.

…crementalForSignedInUsers

Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>

# Conflicts:
#	tests/unit/MiddlewareTest.ts
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I merged the latest main and formatted SaveResponseInOnyx.ts in 311e8a5. The PR no longer has conflicts.

One change from your request: I kept 2 of the 3 imports in MiddlewareTest.ts, not all 3.

  • main changed the only READ_COMMANDS use in this test to SIDE_EFFECT_REQUEST_COMMANDS.SIGN_IN_WITH_SHORT_LIVED_AUTH_TOKEN.
  • With READ_COMMANDS kept, lint fails with 'READ_COMMANDS' is defined but never used.
  • The import is now {SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS}. Both are used by the new test cases.

The formatter fixed a missing space before new Set on SaveResponseInOnyx.ts:28.

Checks: The middleware and user tests pass (67 of 67). Lint passes on both files, and oxfmt makes no other changes. Local npm run typecheck reported 11 errors, all in gesture-handler and Markdown files outside this PR. They come from dependencies that main upgraded, and my local node_modules still had the old versions. CI will run the typecheck against the updated dependencies.

@nyomanjyotisa

Copy link
Copy Markdown
Member

@Julesssss could you please re-review?

Julesssss
Julesssss previously approved these changes Oct 12, 2026
@Julesssss

Copy link
Copy Markdown
Contributor

Sorry @nyomanjyotisa could you resolve blockers by merging main?

@nyomanjyotisa

Copy link
Copy Markdown
Member

@MelvinBot please merge main and resolve the conflicts.

…crementalForSignedInUsers

Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>

# Conflicts:
#	tests/unit/MiddlewareTest.ts
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Merged main into this branch in b32d0b3. The PR no longer has conflicts.

The only conflict was the import line in tests/unit/MiddlewareTest.ts. main reverted the short-lived-token sign-in change, which moved SIGN_IN_WITH_SHORT_LIVED_AUTH_TOKEN back to READ_COMMANDS. This branch still needs SIDE_EFFECT_REQUEST_COMMANDS and WRITE_COMMANDS, so the import now includes all three. SaveResponseInOnyx.ts merged automatically and keeps the revert.

Checks:

  • npm test -- tests/unit/MiddlewareTest.ts tests/actions/UserTest.ts tests/actions/AppTest.ts: 83 passed.
  • npm run lint on the PR's files: passed.
  • npm run typecheck: fails only in tests/actions/IOUTest/SplitTest.ts (missing reportDrafts). This PR doesn't touch that file, and the merged branch matches main outside the PR's four files, so the error comes from main.

@nyomanjyotisa

Copy link
Copy Markdown
Member

@Julesssss main is merged and conflicts are resolved. Could you please re-review?

@Julesssss Julesssss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyomanjyotisa unfortunately there is a failing check now. Could you merge main again and hopefully this is the last time.

@nyomanjyotisa

Copy link
Copy Markdown
Member

@MelvinBot please merge main to pick up the typecheck fix.

…crementalForSignedInUsers

Co-authored-by: Nyoman Jyotisa <nyomanjyotisa@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I merged main into this branch in 0900869, which picks up [No QA] fix ts error on main. The merge had no conflicts.

npm run typecheck passes after the merge. npm test -- tests/unit/MiddlewareTest.ts tests/actions/UserTest.ts also passes (69 of 69).

@Julesssss
Julesssss merged commit e01c311 into main Oct 14, 2026
42 of 43 checks passed
@Julesssss
Julesssss deleted the claude-reconnectAppIncrementalForSignedInUsers branch October 14, 2026 20:05
@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions

Copy link
Copy Markdown
Contributor

🚧 Julesssss has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/Julesssss in version: 9.5.10-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site update is needed. This PR fixes a sync bug so devices that are already signed in keep their chat messages when another device signs in. That matches what users already expect, and no help article describes the old behavior.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/Julesssss in version: 9.5.12-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site update is needed. This PR fixes a sync bug so signed-in devices keep their chat messages when another device signs in, which is what users already expect, and no help article describes the old behavior.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.12-1 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants